fix: js release yml, reduce batch size, change telemetry payload #975
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 6ca47e3 in 9 seconds
More details
- Looked at
62lines of code in3files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. .github/workflows/release_js_sdk.yml:40
- Draft comment:
The removal of the version update and commit steps means the version inpackage.jsonandconstants.jsmust be manually updated before running this workflow. Ensure this is done to avoid publishing with an incorrect version. - Reason this comment was not posted:
Confidence changes required:50%
The removal of the version update and commit steps in the workflow is intentional to avoid permission issues. However, this means the version in package.json and constants.js must be manually updated before running the workflow.
Workflow ID: wflow_Tme0Y6S8ktpeIAQu
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -14,6 +14,8 @@ on: | |||
|
|
|||
| jobs: | |||
| run-js-tests: | |||
| permissions: | |||
| contents: write | |||
There was a problem hiding this comment.
Consider adding a comment explaining why write permissions are needed here. This helps future maintainers understand the security implications and requirements.
js/src/constants.js
Outdated
| @@ -8,7 +8,7 @@ const ACTIONS = { | |||
| // actions list end here | |||
| }; | |||
|
|
|||
| const COMPOSIO_VERSION = `0.3.0`; | |||
| const COMPOSIO_VERSION = `0.4.0-beta`; | |||
There was a problem hiding this comment.
Good to see version alignment between package.json and constants.js. Consider implementing a single source of truth for version numbers to prevent future mismatches. This could be done through a version management script or using package.json as the primary source.
Review SummaryChanges Overview
Positive Aspects✅ Version alignment between package.json and constants.js Suggestions
Overall AssessmentThe changes improve the release process and fix version inconsistencies. The code quality is good, and the changes are focused and purposeful. Approved with minor suggestions for documentation improvements. |
|
This comment was generated by github-actions[bot]! JS SDK Coverage Report📊 Coverage report for JS SDK can be found at the following URL: 📁 Test report folder can be found at the following URL: |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 0f84c29 in 18 seconds
More details
- Looked at
77lines of code in3files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. js/src/sdk/utils/telemetry/index.ts:10
- Draft comment:
Reducing the batch size from 100 to 10 may increase the number of requests sent, potentially affecting performance. Ensure this change is intentional. - Reason this comment was not posted:
Confidence changes required:50%
The batch size in the BatchProcessor has been reduced from 100 to 10. This change might affect performance by increasing the number of requests sent. It's important to ensure this change is intentional and justified.
2. js/src/sdk/utils/error.ts:111
- Draft comment:
The error message forETIMEDOUTshould not mentionECONNABORTED. Consider updating the message to reflect the correct error code. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_qh541wfbqExbNhsw
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 75f1f43 in 9 seconds
More details
- Looked at
36lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/src/sdk/utils/telemetry/index.ts:8
- Draft comment:
The batch size in theBatchProcessorconstructor is set to 100, but the PR description mentions reducing it to 10. Please update the batch size to 10 to match the PR description. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_Wqf5qyJi5YyyV8KS
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on f9512c2 in 10 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_71dHR61YVFaBOVEV
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on ff5ae3b in 25 seconds
More details
- Looked at
94lines of code in3files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/src/sdk/utils/error.ts:90
- Draft comment:
The error message usesECONNABORTEDwhile the error code isETIMEDOUT. Consider updating the error message to reflect the correct error code.
`Request to ${fullUrl} timed out after the configured timeout period. This could be due to slow network conditions, server performance issues, or the request being too large. Error code: ECONNABORTED`,
- Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_7YERFJJ1ZOelhMb1
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
himanshu-dixit
changed the title
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 063b382 in 40 seconds
More details
- Looked at
26lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. js/package.json:3
- Draft comment:
The version update inpackage.jsonto0.4.1-betais inconsistent with the PR description, which mentions0.4.0-beta. Please ensure the version is consistent across the PR. - Reason this comment was not posted:
Comment did not seem useful.
2. js/src/constants.js:11
- Draft comment:
The version update inconstants.jsto0.4.1-betais inconsistent with the PR description, which mentions0.4.0-beta. Please ensure the version is consistent across the PR. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_Ud4epnN1B8wnNKDY
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 4cdbf89 in 30 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/src/frameworks/langchain.spec.ts:72
- Draft comment:
Typo in property name:successfullshould besuccessful. This typo is present in theexpectstatement. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_pUSGqjgThGfubqbs
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Important
Fix permission issues in GitHub Actions, update versioning, improve error handling, and enhance telemetry in the JS SDK.
contents: writepermission torelease_js_sdk.ymlto resolve permission issues.release_js_sdk.yml.package.jsonfrom0.3.2to0.4.1-beta.COMPOSIO_VERSIONinconstants.jsfrom0.3.0to0.4.1-beta.error.tsfromSDK_ERROR_CODES.COMMON.UNKNOWNtoSDK_ERROR_CODES.BACKEND.SERVER_UNREACHABLEfor unreachable server errors.SERVER_UNREACHABLEtoSDK_ERROR_CODESinconstants.ts.telemetry/index.tsfrom 1000 to 10.telemetry/index.ts.This description was created by
for 4cdbf89. It will automatically update as commits are pushed.